gh-144475: Fix a heap buffer overflow in partial_repr#144571
gh-144475: Fix a heap buffer overflow in partial_repr#144571bkap123 wants to merge 18 commits intopython:mainfrom
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Please:
|
|
By the way, @dr-carlos already suggested to open a PR. It is polite to ask them if they want to contribute themselves. As such, I'm going to close this one unless they are fine with you making the PR (we don't really want people "sniping" work of others) |
|
Thanks for the feedback. I missed that @dr-carlos suggested to fix it. I’m happy to close this PR if @dr-carlos is already working on it. |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Hi, thanks for asking! |
|
Here are the changes I made:
|
| Py_DECREF(args); | ||
| Py_DECREF(kw); | ||
| Py_ReprLeave(self); | ||
| return result; |
There was a problem hiding this comment.
I think this return made sense when the label was 'done'.
Do we still want a side-effect of "return" for a label that "frees arguments"?
Or maybe we keep the "done" label?
There was a problem hiding this comment.
I am not exactly sure. I could see arguments for and against adding a done label before the return.
Personally, I don't think it is necessary, as it would never be used. Since the function returns NULL in the case of an error before a new reference is called to the arguments, adding a done label could be a bit misleading. It might suggest that the done label is called if the function needs to return before the fn, args, and kw point to the arguments, as that is the pattern for the other labels.
I would like to get other people's thoughts on this.
There was a problem hiding this comment.
Also, since each of the three labels will only be jumped to on an error, a better solution might be to rename each of the labels to the specific error that occurs rather than what gets freed. That way, it makes more sense why the function returns.
4 participants
Uh oh!
There was an error while loading. Please reload this page.